Remove containerPackageInfo parameter in createOdspCreateContainerRequest
#23872
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area: driver
github-actions
bot
added
changeset-present
public api change
MarioJGMsoft
changed the title
containerPackageInfo parameter in createOdspCreateContainerRequestcontainerPackageInfo parameter in createOdspCreateContainerRequest
MarioJGMsoft
requested review from
a team,
pragya91,
markfields,
jason-ha,
jatgarg,
kian-thompson,
WillieHabi and
rajatch5
and removed request for
a team
There was a problem hiding this comment.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts:35
- With the removal of the containerPackageInfo parameter and its associated URL fragment, please ensure that any parts of the system relying on containerPackageName being sent through createOdspCreateContainerRequest are updated (for example, by using OdspDriverUrlResolverForShareLink as described).
)}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
packages/drivers/odsp-driver/src/test/odspDriverResolverTest.spec.ts:457
- The removal of the containerPackageInfo parameter led to the deletion of this test case. Please confirm that the new behavior (i.e. setting the container package name via OdspDriverUrlResolverForShareLink) is covered elsewhere in the tests.
it("Should create request with containerPackageName and resolve it", async () => {
tylerbutler
previously requested changes
Feb 20, 2025
jason-ha
requested changes
Feb 22, 2025
Comment on lines
-40
to
+35
| )}${containerPackageInfo ? `&containerPackageName=${getContainerPackageName(containerPackageInfo)}` : ""}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`, | ||
| )}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`, |
There was a problem hiding this comment.
This is a breaking behavioral change. Is there a deprecation PR planned before this?
There was a problem hiding this comment.
Yes there is, I'm working on one but I'm having trouble making the deprecation. I'm not sure how to deprecate the parameter within the function.
There was a problem hiding this comment.
You can't deprecate the parameter, but you can deprecate calling the function with that form. Example:
interface Test {
/** @deprecated */
fn(a: string, b: boolean): void;
fn(a: string): void;
}There was a problem hiding this comment.
Created deprecation PR: #23919
|
🔗 Found some broken links! 💔 Run a link check locally to find them. See linkcheck output |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
With PR: #22849 we introduced a parameter for ODSP that will not be used.
Since it is public, we will first mark it as deprecated, and then in the next release remove it. Make sure we inform Loop to not take dependency on it.
This is the follow up task to ensure that the parameter is deleted.
Reviewer Guidance
Please let me know if there's anything that I should change or be aware of. Thanks!
Fixes: AB#31049